Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting redirect_uri at runtime on the callback handler just like it's allowed on the login handler #298

Merged

Conversation

mariano
Copy link
Contributor

@mariano mariano commented Feb 18, 2021

Description

While there is currently a way to specify at runtime the redirect_uri used within the login handler (thus overriding process.env.AUTH0_BASE_URL), the same cannot be done from within the callback handler, resulting in an unauthorized_client bad request error indicating the mismatch between both URLs.

This lack of consistency is evident in login.ts where LoginOptions.authorizationParams.redirect_uri takes precedence over building the redirect URL with urlJoin(config.baseURL, config.routes.callback), while on callback.ts there is no possibility of modification: the redirect url will always be urlJoin(config.baseURL, config.routes.callback).

This PR introduces a new option on CallbackOptions: the optional redirectUri. With it, one can specify a different URL at runtime in [...auth0].tsx by overriding the login and callback handlers. In the following example, assume that the redirect URI is different from AUTH0_BASE_URL, and that its definition might depend on custom headers or other data coming from NextApiRequest, thus rendering the alternative of customizing initAuth0 not viable.

import { handleAuth, handleCallback, handleLogin } from '@auth0/nextjs-auth0';
import { NextApiRequest } from 'next';

export default handleAuth({
  async login(req, res) {
    const baseUrl = "http://messi:3000";
    const options = {
      authorizationParams: {
        redirect_uri: `${baseUrl}/api/auth/callback`
      }
    };

    try {
      await handleLogin(req, res, options);
    } catch (error) {
      res.status(error.status || 400).end(error.message);
    }
  },

  async callback(req, res) {
    const baseUrl = "http://messi:3000";
    const options = {
      redirectUri: `${baseUrl}/api/auth/callback`
    };

    try {
      await handleCallback(req, res, options);
    } catch (error) {
      res.status(error.status || 400).end(error.message);
    }
  }
});

References

Other issues (#108, #154, #295) either directly or indirectly reference this limitation, mostly within the context of a Vercel deployment. We have experienced this ourselves while developing a multi-tenant nextjs auth0 app.

Testing

I've introduced a test case to ensure CallbackOptions.redirect_uri can be specified at runtime.

To replicate this in a real application just set your AUTH0_BASE_URL environment variable to a specific host (for example http://localhost:3000), and then define a different URL in authorizationParams.redirect_uri on the login handler (such as http://messi:3000), which should resolve to the same nextjs app. After logging in on auth0 and being sent back to the nextjs app to handle the oauth callback, it'll consistently fail with a bad request unauthorized_client (The redirect URI is wrong. You sent http://localhost:3000, and we expected http://messi:3000). Applying this PR and utilizing CallbackOptions.redirectUri as shown before solves the problem.

…ndler just as it's currently allowed on the login handler
@mariano mariano requested a review from a team as a code owner February 18, 2021 02:28
@vercel
Copy link

vercel bot commented Feb 18, 2021

@mariano is attempting to deploy a commit to the Auth0 Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@adamjmcgrath adamjmcgrath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just need to add it to the Next handler options

src/auth0-session/handlers/callback.ts Show resolved Hide resolved
@mariano
Copy link
Contributor Author

mariano commented Feb 18, 2021

lgtm, just need to add it to the Next handler options

Just did, sorry I've missed that

@adamjmcgrath adamjmcgrath merged commit 132f403 into auth0:beta Feb 18, 2021
@adamjmcgrath
Copy link
Contributor

Thanks @mariano!

@adamjmcgrath
Copy link
Contributor

D'oh sorry @mariano, could you raise this against main branch? We're not using beta any more

@adamjmcgrath
Copy link
Contributor

@mariano - actually don't worry, have raised #302

@mariano
Copy link
Contributor Author

mariano commented Feb 18, 2021

@mariano - actually don't worry, have raised #302

I was unsure which branch to use; thanks for taking care of it

@mariano
Copy link
Contributor Author

mariano commented Feb 18, 2021

@adamjmcgrath sorry to keep bugging you, but are there plans for releasing a new npm version containing this fix? It is a limitation that prevents multi-tenant nextjs apps from working with auth0, so imho maybe it deserves a release?

I did try forcing package.json to point to the main commit with:

"@auth0/nextjs-auth0": "github:auth0/nextjs-auth0#7d7d4a9c9a8d5aa5bb86556f0dc4b2d7fe2e6ad8"

and while yarn does install it properly it fails during build time while trying to resolve the @auth0/nextjs-auth0 import references

@mariano mariano deleted the runtime-redirect-uri-in-callback-handler branch February 18, 2021 18:58
@adamjmcgrath
Copy link
Contributor

@mariano - no worries, I'll do a release early next week

I did try forcing package.json to point to the main commit with:

You need to build the srcso this wont work, something like the following (untested) would:

$ git clone https://github.com/auth0/nextjs-auth0.git
$ cd nextjs-auth0
$ npm install
$ npm version 1.0.0-mariano.0 --no-git-tag-version # give it a unique version name so your npm install wont hit the cache for `1.0.0`
$ npm pack # this will build the src and package it in a tgz for you
$ cd ../your/next/app
$ npm install ../path/to/auth0-nextjs-auth0-1.0.0-mariano.0.tgz

@mariano
Copy link
Contributor Author

mariano commented Feb 19, 2021

@adamjmcgrath Of course that did it. You are awesome. I don't play with frontend often, so pardon my cluelessness :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants